-
Notifications
You must be signed in to change notification settings - Fork 719
Add NFData instances #11097
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Add NFData instances #11097
Conversation
Oh I did not see this was not marked as |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"cabal.project plugin" looks confusing in the context of the Cabal repository. Please, replace all of its occurrences with "cabal.project HLS plugin" (esp. the changelog).
@ffaf1 I consider the review-needed label more of our manegerial stuff (to notify Matrix, review during the meetings, and such) rather than an interface for contributors. In practice (I find), when people don't want reviews, they put the PR in the draft state (mostly) because they don't know about the label. So, if the PR isn't in the draft state, it's safe to assume that it can be reviewed, I think. |
These instances are also useful for the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree with @ffaf1 but otherwise fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thank you!
Looks like there's a couple of rebase conflicts that need to be fixed. |
@ffaf1 I resolved your older comments: they were straightforward to address and this was done. We can now move forward. |
@@ -226,6 +226,9 @@ data AbiTag | |||
instance Binary AbiTag | |||
instance Structured AbiTag | |||
|
|||
instance NFData AbiTag where | |||
rnf = genericRnf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When Generic
is derived, just instance NFData AbiTag
is sufficient.
Are there reasons to be more explicit?
This PR adds the NFData instances necessary for the HLS plugins to function, as recommended here: haskell/haskell-language-server#4615 (comment)